I tried this out on sunpy and it seems to explode. sunpy/sunpy#7668

It seems that it's not picking up the coverage.xml to upload here: https://github.com/sunpy/sunpy/actions/runs/9445798095/job/26014155532?pr=7668#step:14:16 🤔

tl;dr -- this does not generate patch coverage, right?

Not yet, but I found a way to make it.

tl;dr -- this does not generate patch coverage, right?

Not yet, but I found a way to make it.

For patch coverage we'd need to store the coverage from main somewhere, to compare it to; some people I've seen online have done that by storing it in a hidden page in the repository's wiki, reading and writing it with a github token with ${{ secrets.GITHUB_TOKEN }}

I am trying this out too at spacetelescope/acstools#205 and it still crashes:

https://github.com/spacetelescope/acstools/actions/runs/9463279566/job/26068438067?pr=205

I am trying this out too at spacetelescope/acstools#205 and it still crashes:

https://github.com/spacetelescope/acstools/actions/runs/9463279566/job/26068438067?pr=205

oh whoops, I fixed the artifact issue:

but didn't pass any filenames to coverage combine 😅

Even when CI passes, it failed with the same error.

Coverage XML written to file coverage.xml

mv: cannot stat '.coverage': No such file or directory

Even when CI passes, it failed with the same error.

Coverage XML written to file coverage.xml

mv: cannot stat '.coverage': No such file or directory

it should be uploading / downloading the .coverage database, to be able to combine multiple machine types, so if that's not present then something is messed up

it looks like --cov-report=xml is what's creating the coverage.xml; however, it should still be creating a .coverage database file... I'll so some local testing to see what files it spits out
https://github.com/spacetelescope/acstools/actions/runs/9463279566/job/26118526244#step:10:105

pytest --pyargs acstools --cov-report=xml --cov=acstools /home/runner/work/acstools/acstools/doc --cov --remote-data -v

running pytest --pyargs acstools --cov-report=xml --cov=acstools --cov --remote-data -v locally does appear to create the .coverage file:

❯ ls -a
.							CITATION.md
..							CODE_OF_CONDUCT.md
.bandit.yaml						LICENSE.md
.coverage						MANIFEST.in
.coverage.urelialia2022.local.stsci.edu.35749.XCMKciAx	README.rst
.git							__pycache__
.github							acstools
.gitignore						acstools.egg-info
.mypy_cache						conftest.py
.pytest_cache						coverage.xml
.readthedocs.yaml					doc
.tmp							pyproject.toml
.tox							tox.ini

What is the difference between .coverage and coverage.xml?

What is the difference between .coverage and coverage.xml?

I'm not intimately familiar with it, but as far as I know .coverage is a SQLite database file with support for parallel-mode (storing coverage for multiple machine configurations / runs) while coverage.xml is report-focused and so is expected to only contain results from a single run.

We could use coverage.xml, but then we'd need to have a summary table of coverage for each and every run, instead of a single table collating all runs

Tried to debug at #209 but no luck. I see the coverage ran but the the upload failed: https://github.com/spacetelescope/acstools/actions/runs/9483014271/job/26134042811?pr=205

Tried to debug at #209 but no luck. I see the coverage ran but the the upload failed: https://github.com/spacetelescope/acstools/actions/runs/9483014271/job/26134042811?pr=205

could you also add -a to the ls?

Ah, yes, added. New log: https://github.com/spacetelescope/acstools/actions/runs/9483014271/job/26138000458?pr=205

(my rant about pytest-cov now has its own issue #330)

Phew, took me a while to get there, but I finally have something that works again: zacharyburnett#3

important notes: I couldn't, for the life of me, figure out where pytest-cov would write coverage data¹, nor how it overwrites (as the documentation claims it does) coverage's --parallel-mode, so I had to resort to cutting the middle man again and invoke coverage directly. The obvious drawback is that users need to do the same thing (coverage run --parallel-mode -m pytest ...) from their tox.ini file to leverage this. But since it's a new feature, there's no backward incompatibility, so it should be acceptable, right ?

Oh and I just realized another important limitation : combining coverage data from different platforms is really tricky (TBH, I could never quite figure it out, but I did fiddle with this a couple times and the author claims it's possible). To the extent I understand how it's supposed to work, it should be possible to configure it on the user side.

If we're going to release this in its current state, it should be advertised very clearly.

Whoops. Guess why I didn't run pre-commit hooks locally ? (a clue: #328)

Also, there's a job stuck because it's trying to use macos-12, which isn't available anymore. This is already fixed on main though.

@Cadair how should we proceed to get CI green again ?

pre-commit.ci autofix

Also added an additional change that uses uv to manage the Python installation in the coverage combination step, which appears to work fine in the tests:

Don't suppose you can move that uv commit to the uv PR? I think I'm happy to merge this otherwise?

Don't suppose you can move that uv commit to the uv PR? I think I'm happy to merge this otherwise?

sure!

Thanks everyone!